Skip to content

NocCostHandler class #2724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 33 commits into from
Oct 6, 2024
Merged

NocCostHandler class #2724

merged 33 commits into from
Oct 6, 2024

Conversation

soheilshahrouz
Copy link
Contributor

This PR adds encapsulates functions used to compute NoC cost terms into a class named NocCostHandler. Static variables in noc_place_utils.cpp are converted to member variables of NocCostHandler.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool libarchfpga Library for handling FPGA Architecture descriptions lang-cpp C/C++ code libvtrutil labels Sep 15, 2024
@soheilshahrouz soheilshahrouz changed the title [WIP] NocCostHandler class NocCostHandler class Sep 15, 2024
@soheilshahrouz soheilshahrouz changed the title NocCostHandler class [WIP] NocCostHandler class Sep 15, 2024
@vaughnbetz
Copy link
Contributor

@soheilshahrouz : please resolve conflicts / rebase so I can review just what's new in this PR.

@vaughnbetz
Copy link
Contributor

Also run a NoC QoR run.

@soheilshahrouz soheilshahrouz changed the title [WIP] NocCostHandler class NocCostHandler class Sep 30, 2024
Copy link
Contributor

@AlexandreSinger AlexandreSinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @soheilshahrouz , great PR! I have left some comments.

@github-actions github-actions bot added the lang-python Python code label Sep 30, 2024
Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good; just a few comments. Feel free to merge once CI passes and you make these commenting changes.

This function clears a BlkLocRegistry object. It makes more sense if it is implemented as a BlkLocRegistry method
@soheilshahrouz
Copy link
Contributor Author

Synthetic benchmarks with 16 or more NoC routers

pack time place WL place time total swap place CPD agg BW agg lat latency overrun noc congestion congested links
master 459.6109887 749995.8796 527.2360388 6119522.682 7.259519891 1.74E+07 3.66E-07 0.0000000002222222 0.2341481481 0.6666666667
pr 464.8243264 749995.8796 521.8041735 6119522.682 7.259519891 1.74E+07 3.66E-07 0.0000000002222222 0.2341481481 0.6666666667
Ratio 1.011 1 0.989 1 1 1 1 1 1 1

@vaughnbetz
Copy link
Contributor

Looks good to me. It looks like the only CI failures are the odd/even routing unit tests, which I think you are disabling due to a known problem in them. Is this ready to merge now?

@vaughnbetz
Copy link
Contributor

@soheilshahrouz : ok for me to merge this?

@soheilshahrouz
Copy link
Contributor Author

@vaughnbetz I ran non-odin nightly tests on wintermute and did not see any failures. I think this is ready to be merged.

@vaughnbetz vaughnbetz merged commit 999961a into master Oct 6, 2024
36 of 53 checks passed
@vaughnbetz vaughnbetz deleted the temp_remove_static_vars_noc branch October 6, 2024 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code lang-python Python code libarchfpga Library for handling FPGA Architecture descriptions libvtrutil VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants